Allow custom layers by removing the restriction for non-coarse layers #1631
Conversation
|
@missinglink Can you please review this? |
|
Hi @mansoor-sajjad, I believe the original intent was to avoid showing a mix of administrative data and non-administrative data in the results, unfortunately there are no screenshots of the behaviour at that time but I can imagine a worst-case scenario of something like this: # Search for 'Torstraße 10, Berlin, Germany'
1. Torstraße 10, Berlin, Germany
2. Berlin, Germany
3. GermanyThis isn't the default behaviour we'd like to see in Pelias, as results I haven't tested your PR but I'm guessing that it would revert to the previous behaviour? |
|
Just a thought, but instead of doing https://github.com/pelias/config/blob/master/config/defaults.json#L63 |
This is my understanding as well. The good news is because reverse geocoding in Elasticsearch operates only on points, only admin records with a centroid within 1km of the coordinate in question would ever be shown. So we probably wouldn't see Germany or even Berlin, but we might see
|
…ed non-coarse layers, inorder to support the custom layers for reverse endpoint.
|
Hi, Sorry for the late response. |
mansoor-sajjad
requested review from
missinglink and
orangejulius
and removed request for
orangejulius and
missinglink
There was a problem hiding this comment.
Nice and simple, happy to merge this.
@orangejulius you'll need to remove your block before we can proceed.
|
Hi @mansoor-sajjad, Please see the docs here: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork |
|
Hei @orangejulius, "Allow edits by maintainers" is already checked. ✅ |
👋
We are working the custom sources and layers which are now supported by the Pelias, which is very good and we are excited about that.
We have data from different sources and we have defined the layers which suits our needs and fit nicely with our data.
Here's the reason for this change 🚀
We are testing the custom sources and layers. It works with the
/autocomplete, but it doesn't works with the/reverseendpoint.We have tried to search on this topic and found other issues and discussion regarding this issue.
Specially this one #1161, which pointed us towards the
reverse.jsWe see that the restriction has been added to support only the non-coarse layers in the
/reverseendpoint in the following commit 10b1d28. We have not been able to figure out why it was necessary. @trescube If you remember anything about that change from 2017 please help us.Here's what actually got changed 👏
We have removed that restriction and make the
/reverseendpoint to support all the layers like other endpoints.We have tested the API after that change and it works fine.